[ondilo] Initial contribution by MikeTheTux · Pull Request #18914 · openhab/openhab-addons

MikeTheTux · 2025-07-09T18:40:09Z

[ondilo] Initial contribution

Description

New openHAB binding for Ondilo ICO Pool and Spa monitoring devices, allowing to monitor and automate a pool environment using openHAB’s rules and UI.

Signed-off-by: Michael Weger <weger.michael@gmx.net>

openhab-bot · 2025-07-09T18:49:26Z

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/request-ondilo-binding/98164/7

Signed-off-by: Michael Weger <weger.michael@gmx.net>

lsiepel · 2025-07-09T20:08:55Z

Thanks @MikeTheTux for another contribution from you! As review capacity is somewhat limited, I would like to ask you to perform a self-review by looking at this checklist.

Signed-off-by: Michael Weger <weger.michael@gmx.net>

MikeTheTux · 2025-07-11T19:19:42Z

self review completed. changes checked in ...

Check	Status Self Review
1. proper bundle name in pom.xml ("openHAB Add-ons :: Bundles :: ... Binding")	OK
2. include new bundles in build (main pom.xml) and karaf feature (in src/main/feature of the bundle)	OK
3. NOTICE: EPLv2 license with NOTICE file	OK
4. NOTICE: all dependencies must be listed in NOTICE file	OK (none)
5. README: Based on our template, same sections should be present	OK
6. README: new line after every sentence	OK
7. README: section headers should be capitalized ("Thing Configuration", not "Thing configuration")	OK
8. README: mention all thing type ids, channel ids, configuration parameter keys in the appropriate section	OK
9. i18n: only original language should be provided, translations should be managed in Crowdin.	OK
10. Check copyright year in source file header. It can be auto corrected with mvn -lp : license:format in the root of the openhab-addons project.	OK
11. Thing/Channel labels should be short (<25 chars, max 2-3 words) and capitalized	OK
12. Thing and Channel's should have semantic tags where possible	OK for Things NOK for Channels
13. conservative use of log levels (mainly debug, unless bugs to report or misconfiguration other than on things)	OK
14. handler.initialize() to return fast and set a valid/correct Thing status	OK
15. use of lambdas for runnables	OK
16. handle REFRESH commands	NOK Not implemented as it would causes >10 channel updates in a row during setup (exceeds given API quota) If you want to update the values, use the poll channel instead
17. All conversions of byte[] to String or vice versa should have the Charset specified as well. This includes converting a Input/OuputStream to a Reader/Writer.	OK
18. Sockets and Input/OutputStreams should be used in try-with-resources statements if possible.	OK
19. Make sure that @NonNullByDefault added to every class with the exception of classes with a DTO suffix or in a dto package.	OK
20. Duplicate code should be refactored where possible.	OK
21. In ThingHandler implementations, all asynchronous futures created during initialize should be cleaned up in dispose.	OK
22. Thing config parameters: Specify units where applicable (e.g. unit="s" for seconds)	OK
23. Thing config parameters: Add min/max value where applicable	OK
24. Thing config parameters: Add context tag where applicable (e.g. network-address)	OK
25. Channels declaration: Use Units of Measure (e.g. Number:Temperature)	OK
26. Non-static fields and variables: Use camel case (i.e. no underscores or prefixes in names)	OK
27. Use primitive over complex types (e.g. int vs. Integer)	OK
28. Log stack traces only on severe errors (e.g. detection of a bug)	OK
29. Don't log if a Thing goes offline due to an error. The text passed to updateStatus() is logged by the framework.	OK
30. Use checked exceptions: Extend custom exceptions from Exception	OK
31. Don't throw RuntimeException on expected errors	OK
32. Don't catch Exception unless an external method throws this exception. To handle unexpected expections catch RuntimeException. This can be helpful in repeated scheduled jobs to avoid stopping a refresh task due to temporary failure.	OK
33. Cache result of getConfigAs() and getConfiguration()	OK
34. When creating a thread, declare it as daemon: Thread.setDaemon(true)	N/R
35. Name createad threads with Thread.setName() or via Thread's constructor	N/R
36. Using synchronized on methods in a Handler should be looked at in detail because the parent class uses synchronized already on for example update of status, and this can result in dead locks.	OK
37. Specify representation property in discovery results	OK
38. Cancelling a task (Future): There's no need to check if it have been cancelled already	OK
39. Any compiler warning that can't be avoided should be annotated with @SuppressWarnings	OK
40. Check checkstyle warnings in target/code-analysis/report.html	OK
41. Check formatting issues with mvn spotless:check -Dspotless.check.skip=false	OK
42. JavaDoc can be checked with: mvn javadoc:javadoc.	OK
43. Any code that catches an IOException should also have a special catch for InterruptedIOException	OK
44. Anytime an InterruptedException or an InterruptedIOException is caught, the code must return from the current method as soon as possible unless code is running in a binding managed thread, in which case use your best judgement.	OK

lsiepel

Thansk for performing the self-review. Looked at 16/23 files, will continue later or tomorrow.

CODEOWNERS

bundles/org.openhab.binding.ondilo/README.md

bundles/org.openhab.binding.ondilo/src/main/resources/OH-INF/thing/thing-types.xml

bundles/org.openhab.binding.ondilo/src/main/resources/OH-INF/i18n/ondilo.properties

bundles/org.openhab.binding.ondilo/src/main/resources/OH-INF/addon/addon.xml

...b.binding.ondilo/src/main/java/org/openhab/binding/ondilo/internal/OndiloHandlerFactory.java

Copilot

Pull Request Overview

Initial contribution of a new openHAB binding for Ondilo ICO Pool and Spa monitoring devices, enabling OAuth2 integration, device discovery, and real-time data polling.

Registers the ondilo module in build and feature files
Defines thing types, channels, and translations for the binding
Implements OAuth2 bridge handler, discovery service, handlers, API client, and DTOs

Reviewed Changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
bundles/pom.xml	Registers the `org.openhab.binding.ondilo` module
src/main/resources/OH-INF/thing/thing-types.xml	Defines bridge and device thing types with channels
src/main/java/org/openhab/binding/ondilo/internal/OndiloHandler.java	Handles device polling and channel updates
src/main/java/org/openhab/binding/ondilo/internal/OndiloApiClient.java	Implements OAuth2-enabled HTTP client for Ondilo API
README.md	Provides user documentation and configuration examples

Comments suppressed due to low confidence (3)

bundles/org.openhab.binding.ondilo/README.md:76

The example uses mowerId, which appears copy-pasted; this should reference the pool ID parameter (e.g., id or poolId).

    Thing ondilo "<id_received_from_discovery>" [ mowerId="<id_received_from_discovery>" ] {

bundles/org.openhab.binding.ondilo/src/main/java/org/openhab/binding/ondilo/internal/OndiloApiClient.java:56

[nitpick] Consider adding unit tests for this method to cover HTTP request handling, response parsing, and token refresh logic.

    public synchronized String get(String endpoint) {

bundles/org.openhab.binding.ondilo/src/main/java/org/openhab/binding/ondilo/internal/OndiloHandler.java:157

List does not have a getFirst() method; use recommendations.get(0) to access the first element.

                        Recommendation recommendation = recommendations.getFirst();

bundles/org.openhab.binding.ondilo/src/main/resources/OH-INF/thing/thing-types.xml

Signed-off-by: Michael Weger <weger.michael@gmx.net>

lsiepel

Only the two handlers are left to review

bundles/org.openhab.binding.ondilo/README.md

...penhab.binding.ondilo/src/main/java/org/openhab/binding/ondilo/internal/OndiloApiClient.java

...g.openhab.binding.ondilo/src/main/java/org/openhab/binding/ondilo/internal/OndiloBridge.java

lsiepel

Looked at the remaining files. Many small style comments, but one larger design comment about the seperation fo concerns between the pool, bridge and api classes .

...ab.binding.ondilo/src/main/java/org/openhab/binding/ondilo/internal/OndiloBridgeHandler.java

....openhab.binding.ondilo/src/main/java/org/openhab/binding/ondilo/internal/OndiloHandler.java

...ab.binding.ondilo/src/main/java/org/openhab/binding/ondilo/internal/OndiloOAuth2Servlet.java

Signed-off-by: Michael Weger <weger.michael@gmx.net>

MikeTheTux · 2025-07-12T04:57:46Z

checked and fixed all editorial comments.

Signed-off-by: Michael Weger <weger.michael@gmx.net>

MikeTheTux · 2025-07-12T16:25:03Z

All of the findings are fixed or commented.

The main improvement which is not implemented, is the synchronization of the polling with the expected measure time. Right now, all Things are updated via a single poll-function, ensuring the API rate limit. Polling per Thing would complicate things again ...

Signed-off-by: Michael Weger <weger.michael@gmx.net>

MikeTheTux · 2025-07-12T19:08:24Z

5ae9efb implements:

The binding automatically adjusts the polling schedule to match the expected time of the next measurement, which is typically 1 hour (and 1 minute buffer) after the previous measurement.

Signed-off-by: Michael Weger <weger.michael@gmx.net>

added API to validate recommendations Signed-off-by: Michael Weger <weger.michael@gmx.net>

lsiepel

Thanks for addressing the issues quickly. Besides the comment you had, i also want to add that whenever there is a transient error in the http communcation (like a timeout) the binding does not have some kind of retry mechanism. This might be fine as the polling is rather slow it might also have a big impact.

...g.openhab.binding.ondilo/src/main/java/org/openhab/binding/ondilo/internal/OndiloBridge.java

....openhab.binding.ondilo/src/main/java/org/openhab/binding/ondilo/internal/OndiloHandler.java

...penhab.binding.ondilo/src/main/java/org/openhab/binding/ondilo/internal/dto/LastMeasure.java

...s/org.openhab.binding.ondilo/src/main/java/org/openhab/binding/ondilo/internal/dto/Pool.java

....openhab.binding.ondilo/src/main/java/org/openhab/binding/ondilo/internal/OndiloHandler.java

Signed-off-by: Michael Weger <weger.michael@gmx.net>

lsiepel · 2025-07-13T12:01:21Z

If you are able to fix the open comments before 16:00 it might be part of oh5. Otherwise i'm not sure.

Signed-off-by: Michael Weger <weger.michael@gmx.net>

lsiepel

Thanks for this binding and the quick follow-ups. While there are some minor things to fix, we need to merge now due to the feature freezen. Please create a follow-up PR for post fixes like the warning: #18914 (comment)

LGTM

Now, you could add the binding's logo to the openHAB website. See https://www.openhab.org/docs/developer/addons/#add-your-add-on-s-logo-to-the-openhab-website

* inital checkin of skeleton Signed-off-by: Michael Weger <weger.michael@gmx.net>

* inital checkin of skeleton Signed-off-by: Michael Weger <weger.michael@gmx.net> Signed-off-by: Paul Smedley <paul@smedley.id.au>

MikeTheTux added 2 commits July 3, 2025 12:55

inital checkin of skeleton

86456fa

Signed-off-by: Michael Weger <weger.michael@gmx.net>

first version

94088e6

Signed-off-by: Michael Weger <weger.michael@gmx.net>

MikeTheTux requested a review from a team as a code owner July 9, 2025 18:40

MikeTheTux added the new binding If someone has started to work on a binding. For a new binding PR. label Jul 9, 2025

MikeTheTux added 2 commits July 9, 2025 20:58

editorial changes

84077eb

Signed-off-by: Michael Weger <weger.michael@gmx.net>

spotless ...

aa86621

Signed-off-by: Michael Weger <weger.michael@gmx.net>

self review

f5ab2c7

Signed-off-by: Michael Weger <weger.michael@gmx.net>

lsiepel requested changes Jul 11, 2025

View reviewed changes

lsiepel requested a review from Copilot July 11, 2025 19:30

Copilot AI reviewed Jul 11, 2025

View reviewed changes

bundles/org.openhab.binding.ondilo/src/main/resources/OH-INF/thing/thing-types.xml Outdated Show resolved Hide resolved

MikeTheTux added 2 commits July 11, 2025 22:07

review findings

b167468

Signed-off-by: Michael Weger <weger.michael@gmx.net>

fixed LF

4faa764

Signed-off-by: Michael Weger <weger.michael@gmx.net>

lsiepel requested changes Jul 11, 2025

View reviewed changes

editorial changes from review

c06e7e7

Signed-off-by: Michael Weger <weger.michael@gmx.net>

MikeTheTux added 7 commits July 12, 2025 07:20

review findings

d5074ed

Signed-off-by: Michael Weger <weger.michael@gmx.net>

getInitialAccessToken

11bc68f

Signed-off-by: Michael Weger <weger.michael@gmx.net>

i18n for thingstatus

6b1d062

Signed-off-by: Michael Weger <weger.michael@gmx.net>

i18n

e653b9d

Signed-off-by: Michael Weger <weger.michael@gmx.net>

Pool handler <=> Bridge Handler <=> ApiClient

aa4acba

Signed-off-by: Michael Weger <weger.michael@gmx.net>

recover from BRIDGE_UNINITIALIZED

2edeab6

Signed-off-by: Michael Weger <weger.michael@gmx.net>

bridge -> account

53a8644

Signed-off-by: Michael Weger <weger.michael@gmx.net>

adaptPollingToValueTime()

5ae9efb

Signed-off-by: Michael Weger <weger.michael@gmx.net>

measurement -> measure

547fd11

Signed-off-by: Michael Weger <weger.michael@gmx.net>

buffer increased to 90s, 60s missed the update by some sconds

d45a645

added API to validate recommendations Signed-off-by: Michael Weger <weger.michael@gmx.net>

lsiepel requested changes Jul 13, 2025

View reviewed changes

review findings

5f3e63f

Signed-off-by: Michael Weger <weger.michael@gmx.net>

MikeTheTux added 2 commits July 13, 2025 15:30

review findings

4dc7325

Signed-off-by: Michael Weger <weger.michael@gmx.net>

fixed

ccaad7b

Signed-off-by: Michael Weger <weger.michael@gmx.net>

lsiepel approved these changes Jul 13, 2025

View reviewed changes

lsiepel merged commit 210270f into openhab:main Jul 13, 2025
2 checks passed

lsiepel added this to the 5.0 milestone Jul 13, 2025

This was referenced Jul 13, 2025

Logo for ondilo openhab/openhab-docs#2517

Merged

[ondilo] Implement review findings #18942

Merged

[ondilo] Implement review findings #18955

Merged

phenix1990 pushed a commit to phenix1990/openhab-addons that referenced this pull request Jul 31, 2025

[ondilo] Initial contribution (openhab#18914)

bbfdfea

* inital checkin of skeleton Signed-off-by: Michael Weger <weger.michael@gmx.net>

psmedley pushed a commit to psmedley/openhab-addons that referenced this pull request Aug 6, 2025

[ondilo] Initial contribution (openhab#18914)

1597cd1

* inital checkin of skeleton Signed-off-by: Michael Weger <weger.michael@gmx.net> Signed-off-by: Paul Smedley <paul@smedley.id.au>

Uh oh!

Comments

Conversation

MikeTheTux commented Jul 9, 2025

[ondilo] Initial contribution

Description

Uh oh!

openhab-bot commented Jul 9, 2025

Uh oh!

lsiepel commented Jul 9, 2025

Uh oh!

MikeTheTux commented Jul 11, 2025

Uh oh!

lsiepel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

lsiepel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lsiepel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MikeTheTux commented Jul 12, 2025

Uh oh!

MikeTheTux commented Jul 12, 2025

Uh oh!

MikeTheTux commented Jul 12, 2025

Uh oh!

lsiepel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lsiepel commented Jul 13, 2025

Uh oh!

lsiepel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers